Timing the computations of quality metrics and some optimizations #4308
Conversation
for more information, see https://pre-commit.ci
…o optimize_metrics
for more information, see https://pre-commit.ci
…o optimize_metrics
for more information, see https://pre-commit.ci
…o optimize_metrics
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
Pierre notre héros. |
No name please. |
|
I would also consider that slow computation should be removed of the by default metrics. |
for more information, see https://pre-commit.ci
|
This looks good to me, I've checked the results are the same, and I've drastically reduce the cost of the most important ones. The only remaining extension that is slow is the synchrony, but not sure to understand exactly what it does. Is people needing it? Are also people using the metrics for the drifts @alejoe91 ? Because they could be optimized also |
|
And also, there is something strange, i think, in the implementation of sliding_rpv_violations. At least, this is not what I would have done, and I'm not sure this is also exactly what the authors meant to do, but I could be wrong, I need to read more the paper to double check. I made an implementation that keep results the same (but can gain numba implementation of correlograms, and eventually also of the fast mode available in #4305 ) |
…o optimize_metrics
for more information, see https://pre-commit.ci
…o optimize_metrics
for more information, see https://pre-commit.ci
…o optimize_metrics
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
@alejoe91 I've also drastically improve the drift metric. @alejoe91 @samuelgarcia @chrishalcrow let me know what do you think, but this is now good for me. All metrics have been heavily optimized using the spike vector, and they are much faster now |
|
On real-world data from the IBL, 1140 units for 1000s with ~ 38 millions spikes, the computations of all metrics (including drift) goes from 320 second seconds to only 84 secons with this PR on my laptop. So a x4 speedup, at least, with identical results |
3 participants
After playing with real-size datasets during the hackathon amazingly organized by our "ami du bouchon" @samuelgarcia , I realized, that some persons might consider spikeinterface too slow. And because we want everyong to join the game (the more the merrier), we need to be inclusive. During the hackthon, we've worked to solve the problem with cross-correlograms (see #4305 and #4307 ). However, we might wonder what can be done for the other metrics, and to do so, first thing is to time the metric. This is what is now added by the PR
For a 1000s recording with 1000 units, you can immediatly see some issues, and they are all fixed in this PR. Some metrics, for example, are using suboptimal correlograms (numpy implemetantations), and some others I guess should be optimized. Before, we used to have
{'num_spikes': 21.245169458998134,
'firing_rate': 0.0007224159999168478,
'presence_ratio': 0.14417108400084544,
'snr': 0.040033251003478654,
'isi_violation': 0.049835821002488956,
'rp_violation': 3.409717330003332,
'sliding_rp_violation': 7.334715744997084,
'synchrony': 21.48789839699748,
'firing_range': 0.36698940899805166,
'amplitude_cv': 28.745208095002454,
'amplitude_cutoff': 0.5351223960024072,
'noise_cutoff': 0.8977982619981049,
'amplitude_median': 0.4023318469990045,
'sd_ratio': 48.09409817199776}
With the PR, we now have:
Plus, now rp_violation accepts a unit_ids to avoid recomputations during slices. I'm fairly happy with the PR as it is. It can also be boosted with speed up in CCG. I've also ported the optimizaiton to the drift metric, now really fast